Skip to content

Fix #498: move cartoon Next Action to a persistent CTA#507

Merged
realproject7 merged 4 commits into
mainfrom
task/498-persistent-next-action-cta
Jun 8, 2026
Merged

Fix #498: move cartoon Next Action to a persistent CTA#507
realproject7 merged 4 commits into
mainfrom
task/498-persistent-next-action-cta

Conversation

@realproject7

@realproject7 realproject7 commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • replace the large cartoon next-action cards with a compact persistent CTA bar owned by the right-pane shell
  • remove duplicate top coach/CTA mounts from the preview panel and progress panel so all cartoon workflow routes share one CTA surface
  • add coverage for the compact CTA, the new bottom-shell placement, and the removed preview-panel coach chrome

Verification

  • npm run typecheck
  • npm run lint -- --quiet app/web/components/CartoonNextAction.tsx app/web/components/CartoonNextAction.test.tsx app/web/components/StoriesPage.tsx app/web/components/StoriesPage.test.tsx app/web/components/StoryProgressPanel.tsx app/web/components/StoryProgressPanel.test.tsx app/web/components/PreviewPanel.tsx app/web/components/PreviewPanel.test.tsx
  • npm run app:build
  • npx vitest run --coverage.enabled=false app/web/components/CartoonNextAction.test.tsx app/web/components/StoriesPage.test.tsx app/web/components/StoryProgressPanel.test.tsx app/web/components/PreviewPanel.test.tsx (fails before collection in this environment with Unknown system error -122, write)

Visual / viewport QA notes

  • desktop: the cartoon right pane no longer spends top vertical space on workflow-coach / workflow-context-next-action; the CTA now sits in a dedicated bottom row aligned to the right of the active workflow surface
  • narrow/mobile: the same CTA collapses into a full-width bottom bar row instead of floating over content, so it stays reachable without covering editor handles, cut cards, or save/cancel rows
  • progress, story info, whitepaper, genesis, episodes, and publish now share the same CTA placement because the shell owns it rather than each page rendering its own top card

Risks

  • the shell now decides when file focus should influence CTA state (?focus= only on file-backed routes), so regressions would show up as the wrong episode action surfacing after route switches
  • removing the local top CTA from PreviewPanel and StoryProgressPanel makes StoriesPage the single source of truth, so any future standalone usage of those components would need to opt into shell CTA rendering explicitly

project7-interns
project7-interns approved these changes Jun 8, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

I reviewed PR #507 on live head 6f6129a63e5a74b0073ade106987b30576ca87d3. The change matches issue #498: the large top next-action cards are removed from cartoon workflow pages, and a single persistent CTA is now owned by the right-pane shell so content starts higher while the action remains reachable at the bottom.

Findings

  • No blocking findings.

Decision

Approved on GitHub. StoriesPage now renders the shared bottom CTA surface, PreviewPanel and StoryProgressPanel no longer mount the old top coach chrome, and CartoonNextAction uses ?focus= only for file-backed routes so story-level routes still resolve CTA state from overall progress. Live lint-and-typecheck was still pending during review, so merge should still use the current live head/check state.

project7-interns
project7-interns requested changes Jun 8, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

PR #507 removes the large top CTA surface and moves the compact CTA into the right-pane shell, but the shell-level CTA handler drops the action behavior that used to live inside PreviewPanel. Several Next Action clicks now only select the episode file instead of opening or performing the actionable workspace.

Findings

  • [high] The persistent CTA no longer executes/routs key workflow UI actions. handleWorkflowNextAction maps open-cuts, open-lettering, upload, refresh-assets, and generate-markdown to only handleSelectFile(story, episodeFile). Opening a file leaves PreviewPanel on its default activeTab = "preview", and Genesis Edit still defaults to genesisEditMode = "text", so "Review cuts and start lettering" does not open the cut workspace. Worse, generate-markdown no longer calls the generation endpoint at all; the old local handler called handleGenerateMarkdown(), but now clicking "Prepare the episode for publish" is a no-op if the file is already selected and never posts to /generate-markdown.
    • File: app/web/components/StoriesPage.tsx:1063
    • File: app/web/components/StoriesPage.tsx:1075
    • File: app/web/components/PreviewPanel.tsx:159
    • File: app/web/components/PreviewPanel.tsx:172
    • File: app/web/components/PreviewPanel.tsx:515
    • Suggestion: preserve the old action semantics from PreviewPanel at the shell level: route cut/letter/upload/refresh actions into the target file's Edit/Cuts workspace, execute generate-markdown through the same endpoint/refresh flow, and add StoriesPage-level coverage that clicking the persistent CTA for open-lettering opens cut-list-panel and that generate-markdown posts to the API.

Decision

Requested changes on GitHub. The layout direction matches #498, but the primary CTA must remain functionally actionable after being moved.

project7-interns
project7-interns approved these changes Jun 8, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

I re-reviewed PR #507 on the updated live head 9997a1b0ad2d7e9218f53e39a7c7fc56aac6fdf1. The follow-up restores the CTA action semantics that regressed in the first version: persistent CTA clicks now flow through a one-shot shell request and PreviewPanel applies the same actionable behavior as before.

Findings

  • No blocking findings.

Decision

Approved on GitHub. StoriesPage now forwards one-shot workflow requests instead of collapsing them to file selection only, and PreviewPanel again routes open-cuts / open-lettering / upload / refresh-assets into Edit/Cuts while generate-markdown hits the generation endpoint and refresh flow. The added StoriesPage- and PreviewPanel-level tests cover both flows. Live lint-and-typecheck was still pending during review, so merge should still use the current live head/check state.

project7-interns
project7-interns requested changes Jun 8, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The follow-up restores the missing first-click CTA behavior, but the new one-shot request sequence is not monotonic after a request is handled. That makes subsequent persistent CTA clicks get ignored by the mounted .

Findings

  • [medium] Workflow action requests reuse after the first request is handled and cleared. derives the next from ; once clears the state back to , the next CTA click computes as again. stores the last applied seq in , so the second request with seq exits early and never opens cuts or runs .
    • File:
    • File:
    • File:
    • File:
    • Suggestion: keep a separate monotonic counter/ref for workflow action requests, or include a unique id that is not reset when the request state is cleared. Add coverage for two consecutive persistent CTA clicks after the first is acknowledged.

Decision

Requested changes on GitHub. The original CTA action semantics are mostly restored for the first request, but repeated use of the persistent CTA remains broken until the request id is monotonic.

project7-interns
project7-interns requested changes Jun 8, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The follow-up restores the missing first-click CTA behavior, but the new one-shot request sequence is not monotonic after a request is handled. That makes subsequent persistent CTA clicks get ignored by the mounted PreviewPanel.

Findings

  • [medium] Workflow action requests reuse seq 1 after the first request is handled and cleared. queueWorkflowAction derives the next seq from workflowActionRequest; once onWorkflowActionHandled clears the state back to null, the next CTA click computes the sequence from null and returns 1 again. PreviewPanel stores the last applied seq in appliedWorkflowSeqRef, so the second request with seq 1 exits early and never opens cuts or runs generate-markdown.
    • File: app/web/components/StoriesPage.tsx:146
    • File: app/web/components/StoriesPage.tsx:1071
    • File: app/web/components/StoriesPage.tsx:1074
    • File: app/web/components/PreviewPanel.tsx:545
    • Suggestion: keep a separate monotonic counter/ref for workflow action requests, or include a unique id that is not reset when the request state is cleared. Add coverage for two consecutive persistent CTA clicks after the first is acknowledged.

Decision

Requested changes on GitHub. The original CTA action semantics are mostly restored for the first request, but repeated use of the persistent CTA remains broken until the request id is monotonic.

project7-interns
project7-interns approved these changes Jun 8, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

I re-reviewed PR #507 on the updated live head cca1cc765f2aafe9f2574243e82a10dc65584dfa. The follow-up fixes the remaining repeated-click CTA bug by making workflow request ids monotonic even after handled requests are cleared.

Findings

  • No blocking findings.

Decision

Approved on GitHub. The shell now uses a dedicated monotonic counter for workflow action requests instead of deriving seq from nullable request state, so repeated persistent CTA clicks are no longer ignored by the mounted PreviewPanel. The added StoriesPage coverage exercises two consecutive Next Action clicks after the first request is acknowledged. Live lint-and-typecheck was still pending during review, so merge should still use the current live head/check state.

project7-interns
project7-interns requested changes Jun 8, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The monotonic request id fix itself looks correct, but the updated live head fails the required lint-and-typecheck check. The failure is in the newly added/related CTA action test, so this PR cannot be approved yet.

Findings

  • [high] Live CI is red: npm test fails because the open-lettering workflow request test never reaches the cut workspace. The failing assertion is screen.findByTestId("cut-list-panel") in PreviewPanel.test.tsx; the rendered DOM remains in Genesis Edit / Opening text mode with genesis-edit-mode-text active, so the request is not proving the restored CTA behavior in CI.
    • File: app/web/components/PreviewPanel.test.tsx:143
    • Suggestion: fix the workflow request handling or test setup so an open-lettering request reliably lands in Genesis Edit/Cuts and renders cut-list-panel, then rerun the live required check.

Decision

Requested changes on GitHub until the live lint-and-typecheck check is green on the current head.

project7-interns
project7-interns approved these changes Jun 8, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

I re-reviewed PR #507 on the updated live head 9a918b1c9eedf02be195d8351692e43f910a311c. The follow-up fixes the remaining Genesis reset race that kept the open-lettering CTA path in Opening text mode instead of landing in Edit/Cuts.

Findings

  • No blocking findings.

Decision

Approved on GitHub. PreviewPanel now makes the file-change reset request-aware for cut-oriented workflow actions, preserving genesisEditMode = "cuts" across the actual story/file change without broadly re-running the reset logic later. The updated test coverage is aligned with the live CI failure that blocked the prior head. Live lint-and-typecheck was still pending during review, so merge should still use the current live head/check state.

project7-interns
project7-interns approved these changes Jun 8, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

I re-reviewed PR #507 on the updated live head 9a918b1c9eedf02be195d8351692e43f910a311c. The latest follow-up fixes the Genesis reset race that kept the open-lettering CTA request in Opening text mode, while preserving the monotonic request-id fix from the prior head.

Findings

  • No blocking findings.

Decision

Approved on GitHub. The file-change reset is now request-aware for cut-oriented workflow actions, repeated CTA request ids remain monotonic, and live lint-and-typecheck passes.

@realproject7 realproject7 merged commit d53d3c9 into main Jun 8, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@project7-interns project7-interns project7-interns approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@realproject7 @project7-interns